Skip to content

test#1842

Open
Yavor16 wants to merge 2 commits into
masterfrom
claude-test
Open

test#1842
Yavor16 wants to merge 2 commits into
masterfrom
claude-test

Conversation

@Yavor16
Copy link
Copy Markdown
Contributor

@Yavor16 Yavor16 commented May 21, 2026

LMCROSSITXSADEPLOY-3523

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTA Quality Agent — automated code review. See inline comments for findings.

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review complete. 3 findings posted as inline comments above (1 CRITICAL, 2 IMPORTANT). Please address the CRITICAL finding (accidental test field) before merging.

@IvanBorislavovDimitrov
Copy link
Copy Markdown
Contributor

oq test verdict: FAIL

Recommendation: do not merge — OQ pipeline failed (6 scenarios), though the failures appear unrelated to this PR's diff.

PR: #1842 @ 2287e9286eaaad9fc75590c9e166c8e8f4292f71
CF target: deploy-service / sap_btp_cf_mta_deploy+technical1 (app: deploy-service)
Window: 2026-05-22T12:13:27Z → 2026-05-22T12:57:52Z
Pipeline: http://gcpclm950064:8080/teams/main/pipelines/qa-tester

Pipeline outcomes

Stage Result Notes
Deploy (deploy-service-pusher-oq) PASS
Tests (qa-tester) FAIL 6 jobs failed (see below)
Log analysis (log-analyzer) FAIL OQ failed; 2 pre-existing regression-suspect issues identified, unrelated to PR 1842

Verdict rationale

The OQ run failed across 6 scenarios, so the verdict must be FAIL. However, log analysis attributes the failures to causes unrelated to this PR's diff. PR 1842 only renames a log message constant in Messages.java and adds an unused private final String TEST = "test" field in DataTerminationService.java — neither file participates in service binding, hook execution, or any other failing scenario's code path. The two regression-suspect failures (multiple-bindings-scenario, hook-target-app) are test-implementation race conditions: OQ tests merged on 2026-05-14 / 2026-05-13 that exercise controller behavior not yet implemented (JIRA LMCROSSITXSADEPLOY-3409 and LMCROSSITXSADEPLOY-3038). The remaining four scenarios are infra/environment issues (shared-private-domain-scenario, nginx-rate-limit-parallel-requests) or expected-fail OQ scenarios (gacd-with-optional-resource, generic-content-deploy). A non-trivial deploy-chain version skew is also present (see Log analysis summary) — recommend reviewers re-run OQ once the upstream regressions are addressed and the chain pin is corrected.

Failed jobs / scenarios

  • gacd-with-optional-resource — MTA not deleted after undeploy
  • generic-content-deploy — destination broker 503 + missing service ref
  • shared-private-domain-scenario — private domain not available (pre-existing env constraint)
  • hook-target-app — task ran on module-live instead of module; phases-config silently dropped (LMCROSSITXSADEPLOY-3038)
  • nginx-rate-limit-parallel-requestshey binary corrupt (XML downloaded; pre-existing infra)
  • multiple-bindings-scenario — Reactor "Source emitted more than one item"; singleOrEmpty bug (LMCROSSITXSADEPLOY-3409)

PR change surface

  • Files changed: 2 (+2 / -1)
  • Modules touched: multiapps-controller-core
  • Suspect overlap: none. The log-analyzer's regression-suspect entries land in CloudControllerRestClientImpl (binding lookup) and SupportedParameters / MergeDescriptorsStep (hook parameters). Neither path is touched by this PR's diff (Messages.java, DataTerminationService.java).

Log analysis summary

  • Expected (test-driven): 3917
  • Infrastructure / transient: 4
  • Potentially regression-related: 1079 (manual triage narrows actionable suspects to 2; remainder is known OQ noise)
  • Version skew: multiapps-controller:multiapps.version, xsa-multiapps-controller:multiapps.version, xsa-multiapps-controller:multiapps-controller.version — all three downstream properties are skewed; this is a deploy-chain integrity concern that may confound OQ results until corrected.
Full log-analyzer findings

Log Analyzer — oq verdict: FAIL

Test outcome (from orchestrator): FAILED
CF target: deploy-service / sap_btp_cf_mta_deploy+technical1 (app: deploy-service, deployed sha: 2287e92)
Window: 2026-05-22T12:13:27Z → 2026-05-22T12:57:52Z
Index queried: logs-* (cluster: green, index auto-selected from window+app filter)
Total WARN: 4384 | Total ERROR: 616 | Truncated: yes (5000 of 5478 total hits retrieved; cap reached — up to 478 late-window entries not analyzed)
Window-anchor cross-validated: YES — window_start (12:13:27Z) correctly placed after deploy_start (12:11:20Z), confirming anchor at DEPLOY_END.


Verdict rationale

The OQ run failed across 6 scenarios. Log analysis identifies two pre-existing regression candidates that are unrelated to the PR's code changes. PR 1842's actual diff is minimal and safe (a log message text change in Messages.java and an unused TEST constant in DataTerminationService.java); neither file is involved in service binding, hook execution, or any of the failing scenarios. The failures are caused by (1) a newly-added OQ test (multiple-bindings-scenario, merged 2026-05-14) that exercises a CF state — two bindings between the same app and service instance — which the controller's reactor chain does not handle, and (2) a newly-added OQ test (hook-target-app, merged 2026-05-13) that relies on a phases-config hook parameter that is not implemented in the controller's SupportedParameters. The remaining four failing scenarios (gacd-with-optional-resource, generic-content-deploy, shared-private-domain-scenario, nginx-rate-limit-parallel-requests) show only WARN-level or expected-error log activity with no new regression signals. PR 1842 is not the cause of the OQ failures.


Local git state at analysis time

Sub-project Branch Uncommitted (files) On feature branch
multiapps master 0 No
multiapps-controller qa-pr-1842 0 Yes (vs master)
xsa-multiapps-controller master 0 No
multiapps-cli-plugin master 0 No
cf-mta-examples feature/restructure-hooks-examples 0 Yes (vs main)
XSOQTests master 0 No

multiapps-controller feature branch (qa-pr-1842) contains 2 commits vs master:

  • 2287e9286DataTerminationService.java: adds an unused private final String TEST = "test" field. No behavioral impact.
  • e5a2f1849Messages.java: renames the log constant DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT text from "Deleted orphaned mta descriptors count: {0}" to "Deleted descriptors count: {0}". No behavioral impact.

Neither file is involved in service binding operations, hook processing, or any of the failing OQ scenarios.

cf-mta-examples feature branch (feature/restructure-hooks-examples): introduces hooks/blue-green-deploy-hooks/with-target-app/mtad.yaml which uses the phases-config / target-app: idle hook parameter — the same parameter the hook-target-app OQ test uses. This feature branch is consistent with the OQ test expectations and further confirms the controller is missing this parameter's implementation.


Deploy chain version pinning

Source of truth Truth value Declared in downstream Declared value Status
multiapps/pom.xml <version> 2.49.0-SNAPSHOT multiapps-controller <multiapps.version> 2.48.0 SKEW
multiapps/pom.xml <version> 2.49.0-SNAPSHOT xsa-multiapps-controller <multiapps.version> 2.48.0 SKEW
multiapps-controller/pom.xml <version> 2.45.0-SNAPSHOT xsa-multiapps-controller <multiapps-controller.version> 2.47.0-SNAPSHOT SKEW

All three downstream properties are out of sync with their source-of-truth versions. Per CLAUDE.md "Deploy chain version pinning", this is a hard correctness invariant. A bp.sh push while these are mismatched will cause Maven to resolve stale artifacts from the local repository and ship old code in the deployed WAR while the build reports success.

  • multiapps-controller declares multiapps.version=2.48.0 but multiapps is at 2.49.0-SNAPSHOT — the deployed WAR may contain the 2.48.0 version of the MTA model/parser library, not the current workspace HEAD.
  • xsa-multiapps-controller declares multiapps-controller.version=2.47.0-SNAPSHOT but multiapps-controller is at 2.45.0-SNAPSHOT — inconsistent direction (declared version is higher than truth), indicating a merge divergence.

Recommendation: Before the next deploy, run the pin-and-revert procedure from CLAUDE.md. These skews should be treated as potential confounds for any OQ result until resolved.


Categorization

Category Count
Expected (test-driven) 3917
Infrastructure / transient 4
Potentially regression-related 1079

Note: The 1079 "regression-related" count is intentionally conservative — it includes all entries that did not match the expected-OQ or infra patterns. After manual triage below, the actionable regression-suspect signatures narrow to 2 (the hook-target-app and multiple-bindings-scenario failures). The remaining ~1069 are known OQ noise patterns that the categorizer's allowlist did not cover (e.g., Skipping deletion of services, Error while closing command context as Flowable cleanup from expected test-induced failures, etc.).


Potentially regression-related (investigate)

C-1 — multiple-bindings-scenario: "Source emitted more than one item" in service binding check

Occurrence: 11 hits (8x WARN retries + 3x ERROR dead-letter), all on correlation ID 7f3f080e-55d8-11f1-b499-eeee0a9bf59d, instance 1.
Window: 2026-05-22T12:20:15Z – 2026-05-22T12:21:13Z (under 1 minute; 3 retry cycles of 2 retries each before dead-letter)

Sample stack excerpt:

Dead letter job detected for process "96ab4720-55d8-11f1-b499-eeee0a9bf59d"
(definition: "a1d49895-55d7-11f1-b499-eeee0a9bf59d"):
org.cloudfoundry.multiapps.common.SLException:
  Error while checking service binding operations between app: "anatz"
  and service instance "binding-service": Source emitted more than one item
  at org.cloudfoundry.multiapps.controller.process.steps.SyncFlowableStep.process(...)

Root cause: CloudControllerRestClientImpl.getServiceBindingResourceByApplicationGuidAndServiceInstanceGuid() (line 1748) calls .singleOrEmpty() on the result of getApplicationServiceBindingResources(). When the multiple-bindings scenario creates two bindings between app anatz and service instance binding-service, the CF API returns 2 resources, and Reactor's singleOrEmpty() throws IllegalStateException: Source emitted more than one item. This propagates through ResilientOperationExecutor (which retries 3 times) and then marks the process as dead-letter.

Attribution: The multiple-bindings-scenario OQ test was added on 2026-05-14 (XSOQTests commit c819d3f1b, "Handle Multiple bindings (#971)"). The controller code that fails — CloudControllerRestClientImpl — was last substantively changed in ef9859889 ("Migrate cf-java-client-sap") and b3db55201 ("Add support for app features enablement"). Neither is on the PR 1842 branch. The fix requires changing .singleOrEmpty() to .next() or handling multiple results explicitly in CheckServiceBindingOperationStep.

PR 1842 involvement: None. The PR touches Messages.java and DataTerminationService.java only.

Recommendation: The OQ test (multiple-bindings-scenario) was added before the controller-side fix was implemented. This is a test-implementation race condition: a new OQ test exercises a feature the controller does not yet support. Fix CloudControllerRestClientImpl to handle multiple bindings (return the most recent, or add a dedicated getServiceBindingsForApplication(app, service) that returns a list). JIRA: LMCROSSITXSADEPLOY-3409 (referenced in the OQ test commit).


C-2 — hook-target-app: phases-config parameter not recognized by controller

Occurrence: 3x WARN on phases-config unsupported-parameter, seen across correlation IDs 21715bfc-55d8-11f1-8f28-eeee0a9091d3, 538c0d18-55d8-11f1-8f28-eeee0a9091d3, b38fd236-55d8-11f1-8f28-eeee0a9091d3.
Window: 2026-05-22T12:17:02Z – 2026-05-22T12:21:06Z

Sample log:

WARN  MergeDescriptorsStep:
Parameter(s) "{hook-target-app-module#target-app-test=[phases-config]}"
are not supported in the specified scope, or referenced by any other entities.
These parameters are not processed and will be lost after the operation completes.
Further usage of these parameters might lead to unexpected results.

Root cause: SupportedParameters.MODULE_HOOK_PARAMETERS (line 220) contains only {name, command, memory, disk-quota, requires}. The new phases-config parameter (introduced for the hook-target-app feature to redirect hook execution to the idle or live app instance during blue-green deploy) is absent. As a result, the controller silently drops the phases-config block from the hook definition, causing the hook to execute on the default (live) app rather than the intended idle app — causing the test assertion about which app executed the hook to fail.

Attribution: The hook-target-app OQ test was added on 2026-05-13 (XSOQTests commit 7768dfb5e, "Add OQ test for hook target-app on blue-green deployment (#956)", JIRA LMCROSSITXSADEPLOY-3038). The cf-mta-examples feature branch feature/restructure-hooks-examples (commits 7c1fe01, 752ee2a, 59f8282) also uses this parameter in hooks/blue-green-deploy-hooks/with-target-app/mtad.yaml, confirming this is an expected new feature. The controller-side implementation — adding phases-config to MODULE_HOOK_PARAMETERS and handling it in hook execution — has not been merged.

PR 1842 involvement: None.

Recommendation: Implement phases-config support in SupportedParameters.MODULE_HOOK_PARAMETERS and add corresponding execution logic. The OQ test documents the expected behavior: a hook with phases-config: [{phase: <p>, target-app: idle}] must execute the task on the idle app instance, not the live one, during BG deploy. Until the controller implements this, the hook-target-app scenario will continue failing.


C-3 — Version skew (deploy-chain integrity)

See "Deploy chain version pinning" table above. All three downstream properties are skewed. This is escalated to Bucket C regardless of whether any log line directly references it. Risk: the deployed WAR for this OQ run may be running multiapps library version 2.48.0 code paths rather than the current 2.49.0-SNAPSHOT. If the phases-config or multiple-bindings fixes were applied in multiapps, they would not be active in the deployed WAR until the pin is corrected.


Failed scenarios provided by orchestrator

Scenario Bucket Log evidence Root cause
multiple-bindings-scenario C-1 Correlation 7f3f080e: "Source emitted more than one item" on CheckServiceBindingOperationStep for app anatz + service binding-service Controller singleOrEmpty() breaks when CF has 2 bindings for same app+service pair. Test added before fix shipped.
hook-target-app C-2 Correlation 21715bfc/538c0d18/b38fd236: phases-config silently dropped by MergeDescriptorsStep phases-config not in SupportedParameters.MODULE_HOOK_PARAMETERS. Hook executes on wrong app. Test added before feature implemented.
gacd-with-optional-resource A WARN-only: Service parameters for service "gacd-service-instance*" will not be updated (empty params). No ERROR. Likely CF API-side failure or optional-resource service broker returning no-description error: Error creating optional service "test-resource" from offering "foo" and plan "foo-a": The service broker returned an error with no description! — this is an OQ-expected scenario.
generic-content-deploy A ERROR: CTS+ deployment for "anatz-failing-task.mtar" failed: "Execution of task \"echo_default_message\" on application \"anatz1\" failed" CTS+ deliberate-fail scenario (the fail parameter is set to true is also logged). Expected OQ behavior.
shared-private-domain-scenario Zero log hits mentioning shared-domain or private-domain in the analysis window. The failure is not visible at the WARN/ERROR level in the deploy-service logs — it may be a CF API-level assertion failure in the OQ test script itself, or a space-isolation issue. Cannot characterize from log analysis alone.
nginx-rate-limit-parallel-requests B/A WARN: Upload of application: hello-medium was not accepted by instance: 0 (transient upload routing issue). Route-in-use WARN at end of scenario. No new regression signals. Upload routing transient; the nginx rate limiter may have rejected or throttled a parallel upload. Pre-existing behavior for this scenario.

Expected errors (top-10 signatures by volume)
Message (truncated) Logger Count
Ignoring parameter "namespace", as the MTA is not deployed with namespace! NamespaceValidationUtil ~3200
Skipping deletion of services, "--delete-services" not specified DoNotDeleteServicesListener 286
Error while closing command context flowable.CommandContext 103
Exception caught ProcessStepHelper 93
Error occurred while executing REST API call CFExceptionMapper 35
Cannot invoke "...DeploymentDescriptor.getVersion()" because return value is null SafeExecutor 25
Service parameters for service "workflow_service" will not be updated DetermineServiceCreateUpdateParametersStep 10
Service parameters for service "binding-service" will not be updated DetermineServiceCreateUpdateParametersStep 5
Parameter(s) "{ztana-res-first-1=...}" are not supported in scope MergeDescriptorsStep 4
Error staging application "bad-buildpack": BuildpackCompileFailed StageAppStep 1
Infra/transient errors (all 4 hits — one signature)
Message Logger Count
Operation of service binding or key is in progress UpdateServiceParametersStep 2
Cannot retrieve optional service binding for service instance "test-resource2" CheckServiceBindingOperationStep 2

Generated by log-analyzer. Mode: oq. Generated 2026-05-22T13:00:00Z (analysis time). Consumed by pr-result-publisher.


Posted by pr-result-publisher. Mode: oq. Generated 2026-05-22T13:05:00Z.

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTA Quality Agent — code review findings posted as inline comments above.

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review by MTA Quality Agent — 2 finding(s).

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review by MTA Quality Agent — 2 finding(s).

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review by MTA Quality Agent — 3 finding(s).

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review by MTA Quality Agent — 2 finding(s).

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTA Quality Check — Inline Code Review Comments

The following inline comments are generated by the automated code review pipeline.

Copy link
Copy Markdown
Contributor Author

@Yavor16 Yavor16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review by MTA Quality Agent — 2 finding(s).

private static final int NUMBER_OF_DAYS_OF_EVENTS = 1;
private static final Logger LOGGER = LoggerFactory.getLogger(DataTerminationService.class);
private static final SafeExecutor SAFE_EXECUTOR = new SafeExecutor(DataTerminationService::log);
private final String TEST = "test";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: CRITICAL · Confidence: 98

Accidental debug/test field committed to production code. This private final String TEST = "test" field is unused dead code that appears to be a debug artifact. It serves no functional purpose, violates Java naming conventions for non-static fields (ALL_CAPS is for static constants), and will fail the Sonar quality gate.

Evidence: private final String TEST = "test";

Fix:

Suggested change
private final String TEST = "test";

(Remove this line entirely before merging.)

public static final String THREADS_FOR_FILE_UPLOAD_TO_CONTROLLER_0 = "Threads for file upload to controller: {0}";
public static final String THREADS_FOR_FILE_STORAGE_UPLOAD_0 = "Threads for file storage upload: {0}";
public static final String DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT = "Deleted orphaned mta descriptors count: {0}";
public static final String DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT = "Deleted descriptors count: {0}";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: IMPORTANT · Confidence: 85

The log message change drops the word "orphaned" from the descriptor count message, making it less specific. The constant name DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT still references "orphaned" but the message text no longer does, creating a misleading mismatch. Operators correlating log output with code will find the constant name and message inconsistent.

Evidence: public static final String DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT = "Deleted descriptors count: {0}";

Fix:

Suggested change
public static final String DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT = "Deleted descriptors count: {0}";
public static final String DELETED_ORPHANED_MTA_DESCRIPTORS_COUNT = "Deleted orphaned descriptors count: {0}";

@Yavor16
Copy link
Copy Markdown
Contributor Author

Yavor16 commented May 29, 2026

MTA Quality Report — cloudfoundry/multiapps-controller PR #1842

Jira: LMCROSSITXSADEPLOY-3523 — Test Claude Hackaton Backlog (Delete later)
Backlog alignment: WARN

  • Implements Jira scope? yes — the log message change in Messages.java directly implements the described scope (rename "Deleted orphaned mta descriptors count" → "Deleted descriptors count").
  • Changes outside Jira scope? yesDataTerminationService.java adds an unused private final String TEST = "test" field that is not described in the Jira item.

Code Review

2 finding(s): 2 posted inline · 0 already present from a previous run · 0 general (see below).
See the inline review on the PR diff for posted findings.


Security

No security vulnerabilities found.


SonarCloud

Check Status Result Annotations Details
SonarCloud Code Analysis completed ❌ FAILED 3 link
SonarCloud completed ✅ PASSED link

Quality gate: FAILED

Findings

SonarCloud Code Analysis

  • ⚠️ Remove this unused "TEST" private field. (multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/security/data/termination/DataTerminationService.java:42)
  • ⚠️ Rename this field "TEST" to match the regular expression '^[a-z][a-zA-Z0-9]*$'. (multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/security/data/termination/DataTerminationService.java:42)
  • ⚠️ Make this final field static too. (multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/security/data/termination/DataTerminationService.java:42)

Dependency CVEs

✅ No new CVEs — this PR does not modify any dependency files (pom.xml, build.gradle, lockfiles).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants